fix: coalesce split Myers hunks to prevent false merge conflicts#2476
Conversation
|
Sebastian Thiel (@Byron) I think the journey test failure is flaky? Also, as far as I can tell this analysis doesn't contain any contradictions, and the .svelte based test cases pass correctly with this patch applied. |
Simon Larsén (slarse)
left a comment
Simon Larsén (slarse)
left a comment
There was a problem hiding this comment.
Hello! I took the liberty of analyzing the test case and problem. See way too long comment :)
When imara-diff's Myers algorithm diffs two files, it sometimes splits what is logically one change into a non-empty deletion hunk and a separate empty insertion hunk, with one unchanged base line between them. This is a valid minimal edit script, but it differs from the alignment that git's xdiff (also Myers-based) would choose. When the empty insertion lands at a base position that the other side of a 3-way merge also touches, `take_intersecting` reports an overlap and the merge produces a conflict — even though `git merge-file` resolves the same inputs cleanly. Fix this by adding a pre-processing step after sorting hunks: for each empty-range insertion hunk, look backwards for the nearest same-side non-empty hunk within a gap of at most one unchanged base line. If found, extend that hunk to cover the gap and the insertion point. This re-joins the split hunk, making the merge robust to different diff algorithm alignment choices. The coalescing is conservative: it only applies when (a) the insertion hunk has an empty before-range, (b) there is a same-side non-empty hunk nearby (gap ≤ 1), and (c) that hunk is the nearest same-side hunk. This avoids affecting cases like zdiff3-interesting where empty insertions are standalone and represent genuinely different changes. Closes GitoxideLabs#2475 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This can lead to cleaner, more Git-like merges.
Sebastian Thiel (Byron)
force-pushed
the
fix/false-conflict-empty-insertion-overlap
branch
from
9b65de7 to
91feaf2
Compare
| hunks.extend(imara_diff::Diff::compute(algorithm, input).hunks().map(|hunk| Hunk { | ||
| let mut diff = imara_diff::Diff::compute(algorithm, input); | ||
| diff.postprocess_lines(input); | ||
| hunks.extend(diff.hunks().map(|hunk| Hunk { |
There was a problem hiding this comment.
This is now the actual fix, which was enabled by migrating to imara-diff v0.2 which comes with this kind of post-processing.
This also was facilitated by finally vendoring it, so the entire stack for diffing can be manipulated more swiftly and easily.
|
And thanks again for making this happen Mattias Granlund (@mtsgrd), and my apologies for taking this long. While ultimately taking the long route turned out to be right, it's also a bit me being afraid of these things. |
Sebastian Thiel (Byron)
merged commit 172bd22
into
GitoxideLabs:main
3 participants
Summary
coalesce_empty_insertions_with_nearest_same_side_hunk()as a pre-processing step after sorting hunks, before the intersection checkfalse_conflict::myers_false_conflict_with_blank_line_ambiguity) — 5/4/4 lines of generic textContext
Reported in #2475. When GitButler amends a commit via
create_tree()→merge_trees(), certain edits produce aCherryPickMergeConflictthatgit merge-fileresolves cleanly.Root cause
Three-way merge of:
base
base → ours (delete
alpha_x, add a blank, remove trailing blank):base → theirs (delete
bravo_x):alpha_x (blank) - bravo_x charlie_x (blank)Myers (imara-diff) diffs base→ours as three hunks:
alpha_xHistogram diffs it as two hunks:
alpha_x→ blankTheirs (both algorithms):
before=2..3 after=2..2(deletebravo_x).The empty insertion at position 2 (Myers hunk #2) collides with theirs' deletion at 2..3 via the empty-range special case in
left_overlaps_right, producing a false conflict.Fix
After sorting hunks by ancestor position, a new
coalesce_empty_insertions_with_nearest_same_side_hunk()pass absorbs empty insertions into their nearest preceding same-side non-empty hunk (gap ≤ 1). This turns Myers hunks #1 + #2 into a single{before=0..2, after=0..2}hunk that no longer overlaps with theirs at position 2.The coalescing is conservative — it only fires when (a) the hunk has an empty before-range, (b) a same-side non-empty hunk exists within 1 base line, and (c) that hunk is the nearest same-side hunk. This avoids affecting cases like
zdiff3-interestingwhere standalone empty insertions represent genuinely different changes.Test plan
myers_false_conflict_with_blank_line_ambiguitypassesgix-mergetest suite passes (22/22), includingrun_baselineandtree::run_baselinegit merge-file -pconfirms clean merge on the same inputsCloses #2475
🤖 Generated with Claude Code